Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Localization example #293

Merged
merged 3 commits into from
Apr 15, 2022
Merged

Localization example #293

merged 3 commits into from
Apr 15, 2022

Conversation

serwizz
Copy link
Contributor

@serwizz serwizz commented Apr 6, 2022

No description provided.

@serwizz serwizz mentioned this pull request Apr 6, 2022
@@ -54,3 +55,12 @@
'XsdGlobals', 'XMLSchemaBase', 'XMLSchema', 'XMLSchema10', 'XMLSchema11',
'XsdComponent', 'XsdType', 'XsdElement', 'XsdAttribute',
]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translation install should be activable by option or by an utility function after xmlschema import.
This because someone might not want to turn on translation or might want to use another custom translation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think about some settings option or environment variable. But all strings in code must be wrapped in _() function, which initializing in translation.install(). And I don't know how make translation optionable. Maybe language code should be in settings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a library and currently it doesn't have settings, and I would prefer to avoid this in order to keep it simple.

As i saw installed translations are stacked into gettext module internals, so i think that a reasonable manner to handle this is to provide an utility to add package translations, also parametrized with language codes. I don't know how currently, this could be decided after looking at solutions used in other packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add initialization via schema = xmlschema.XMLSchema11(xsd, use_translation=True)

@@ -0,0 +1,94 @@
# SOME DESCRIPTIVE TITLE.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add copyright info on this part.
If it's not a problem also add author info.

Copy link
Contributor Author

@serwizz serwizz Apr 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, it was autogenerated by xgettext utility

@@ -0,0 +1,94 @@
# SOME DESCRIPTIVE TITLE.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A script or an utilily function for regenerate this POT file could be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe need adding Localization section in README with helpful utilities. In fact they may be not easy to use.

# generate *.pot file with all strings, that must be translated
xgettext xmlschema/validators/simple_types.py xmlschema/validators/exceptions.py -o xmlschema/locale/xmlschema.pot
# update locale *.po file from *.pot
msgmerge xmlschema/locale/ru/LC_MESSAGES/xmlschema.po xmlschema/locale/xmlschema.pot -o xmlschema/locale/ru/LC_MESSAGES/xmlschema.po
# generate *.mo file from locale *.po
msgfmt xmlschema/locale/ru/LC_MESSAGES/xmlschema.po -o xmlschema/locale/ru/LC_MESSAGES/xmlschema.mo

return translator.gettext(message)


_ = get_translation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to use locale/__init__.py for this, but instead of providing extra arguments to schema class it could be better to provide an helper function like this:

import gettext
from pathlib import Path

translation = None


def install_translation(localedir=None, languages=None, class_=None, fallback=False):
    global translation

    if localedir is None:
       localedir = pathlib.Path(__file__).parent..joinpath('../locale').resolve()

    translation = gettext.translation(
        domain= 'xmlschema',
        localedir=localedir,
        class_=class_,
        fallback=fallback,
    )
    translation.install()

importing install_translation into package __init__.py the function can be used to activate xmlschema provided translations after package import:

import xmlschema
xmlschema.install_translation()
schema = xmlschema.XMLSchema(...)

or to install other custom translations for the xmlschema domain providing optional arguments:

import xmlschema
xmlschema.install_translation(localedir='../mylocale', languages=['fur'])
schema = xmlschema.XMLSchema(...)

@@ -321,11 +323,19 @@ def __init__(self, source: Union[SchemaSourceType, List[SchemaSourceType]],
build: bool = True,
use_meta: bool = True,
use_fallback: bool = True,
loglevel: Optional[Union[str, int]] = None) -> None:
loglevel: Optional[Union[str, int]] = None, use_translation=False,
translation_folder: PurePath = None) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need to keep the translation setting at each schema init you can collapse to a single extra argument, eg.:

use_translation: Union[None, Path, str] = None)

@brunato brunato marked this pull request as ready for review April 15, 2022 10:11
@brunato
Copy link
Member

brunato commented Apr 15, 2022

Hi @serwizz,

If you don't have other updates to push i can merge the PR and then rework from this base to do other changes (i have to add at least an italian translation and prepare an helper script for updating a language translation).

Thank you

@serwizz
Copy link
Contributor Author

serwizz commented Apr 15, 2022

Hi,

Yes, you can merge. I do not insist on this implementation, it just an expample. After merge I can continue wrapping messages to _() function and adding translation.

Thanks

@brunato
Copy link
Member

brunato commented Apr 15, 2022

Ok, i will try to rework as I wrote in comments, and also writing some tests for checking the translations.

Thanks

@brunato brunato merged commit c0363bc into sissaschool:master Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants